Fix#139 "Process exited with code 1" #463
Conversation
…s Python Add environment_validator.py that validates Python version (>= 3.12) and dependency imports in both bundled and venv interpreters. Features: - Validates Python version meets minimum requirement (3.12+) - Checks core dependencies (claude_agent_sdk, dotenv, pydantic) - Checks optional dependencies (real_ladybug, graphiti_core, google-generativeai) - Supports both macOS and Windows path detection - Provides CLI with --check-bundled, --check-venv, --json, --verbose flags - Returns detailed error messages for troubleshooting This addresses GitHub Issue AndyMik90#139 by providing environment validation to prevent "Process exited with code 1" errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…build tools Implements SystemChecker class that validates build tool availability (make, cmake) with platform-specific requirements and installation instructions: - macOS: Requires cmake (make comes with Xcode CLT) - Windows: Requires both make and cmake - Linux: Requires cmake (make usually pre-installed) Features: - Tool version detection and path discovery - Clear installation instructions per platform - JSON output mode for programmatic use - Verbose mode with detailed tool status - CLI interface with --platform flag 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…requirements.txt to both bundled and venv Python interpreters - Implements DependencyInstaller class with methods for: - install_to_interpreter(): Install to any Python interpreter - install_to_bundled(): Install to bundled Python - install_to_venv(): Install to venv Python - install_to_both(): Synchronize both interpreters - verify_synchronization(): Check dependency sync without installing - CLI interface with flags: - --verify-only: Check sync status without installing - --install-all: Install to both interpreters - --install-bundled / --install-venv: Target specific interpreter - --json: JSON output for programmatic use - --verbose: Show detailed package lists - Platform support for macOS, Windows, and Linux path detection - Detailed error reporting with stdout/stderr capture - Follows patterns from environment_validator.py and system_check.py 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ython-env-manager Add validateBothEnvironments() method to PythonEnvManager class that: - Calls backend environment_validator.py with --check-bundled --check-venv --json - Parses JSON output to extract validation results for both interpreters - Returns structured result with success status, errors, and summary - Handles cases where validator script is not found (skips gracefully) - Handles exec errors and parses stdout even on non-zero exit codes This enables the frontend to validate that both bundled Python and venv Python environments have all required dependencies installed before running features like Insights, Roadmap, and Ideation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…env-manager Add validateBuildTools() method that calls system_check.py before pip install to verify build tools (make, cmake) are available. This prevents cryptic build failures when compiling native packages like real_ladybug. Features: - Calls backend system_check.py with --json flag for structured output - Returns missing tools list with platform-specific installation instructions - Gracefully handles missing validator script (doesn't block) - Supports both dev mode and packaged app paths - Follows existing validateBothEnvironments() pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Combine both stderr and stdout for comprehensive error output - Include exit code in error message (e.g., "pip install failed (exit code 1)") - Truncate to last 2000 chars to capture actual error while avoiding overly long messages - Provide detailed pip output in error message shown to users 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…n status - Add ENV_VALIDATE_STATUS, ENV_VALIDATE_START IPC channel constants - Add ENV_VALIDATE_PROGRESS, ENV_VALIDATE_COMPLETE, ENV_VALIDATE_ERROR events - Create environment-handlers.ts with registerEnvironmentHandlers function - Integrate handlers with PythonEnvManager for validation - Export EnvironmentValidationStatus type for renderer use 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ys validation progress and errors - Created StartupValidator.tsx component that displays environment validation progress during app startup - Shows step indicators for Build Tools and Python environment validation phases - Displays error messages with actionable installation instructions (brew, choco, apt-get) - Includes copy-to-clipboard functionality for installation commands - Supports reduced motion preferences for accessibility - Created environment-api.ts preload module with IPC bindings for validation API - Added EnvironmentAPI to preload index exposing startValidation, getValidationStatus, and event listeners - Follows existing RoadmapGenerationProgress and GlobalDownloadIndicator patterns 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…lization Integrate environment validation into app startup to check build tools (make, cmake) and Python environments on first project load. Changes: - Import StartupValidator component - Add environment validation state with sessionStorage persistence - Trigger validation on first project load (when projects exist and project is selected, but validation hasn't run yet) - Add handlers for validation completion (persists to sessionStorage) and skip (allows bypass without persistence) - Render StartupValidator as overlay with backdrop blur when validating The validator: - Shows automatically on first project load - Persists success to sessionStorage (clears on app restart) - Allows skipping (will show again on next app launch) - Blocks UI with overlay until validation completes or is skipped 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add stderrOutput variable to capture stderr separately - Include actual Python error message in error output instead of just exit code - Keep last 5KB of stderr for error messages to avoid memory issues 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…te start Add EnvValidationLogger class that writes startup validation logs to ~/Library/Logs/auto-claude/env-validation.log with: - Session headers with platform, arch, node version info - Timestamped log entries with INFO/WARN/ERROR levels - Python path discovery logging - Validation step results (PASS/FAIL) - Validation summary Integrates logging into: - validateBothEnvironments() - logs validator discovery and results - _runEnvironmentValidator() - logs execution and validation results - _doInitialize() - logs initialization steps and outcomes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add comprehensive unit tests for the environment validator module: - DependencyStatus, ValidationResult, DualValidationResult dataclasses - Python version detection (3.12+ validation, old versions, errors) - Dependency checking (installed, missing, dotted module names) - Path detection functions (bundled/venv paths per platform) - Full environment validation flow - Dual environment validation Tests use mocking to avoid subprocess calls and ensure determinism. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add comprehensive unit tests for system_check.py module: - Platform requirements tests (Windows requires make+cmake, macOS/Linux require cmake only) - Installation instructions tests (Homebrew for macOS, Chocolatey for Windows, apt for Linux) - SystemChecker initialization and configuration tests - Tool detection tests with mocked subprocess calls - Version extraction tests from various tool output formats - Error handling tests for timeouts, missing tools, and edge cases Tests verify make/cmake detection logic works correctly for Windows and macOS platforms. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…m startup Add comprehensive integration tests for the startup validation E2E flow: - TestEnvironmentValidatorIntegration: Tests for environment_validator.py including module imports, dataclass initialization, and validation logic - TestDualEnvironmentValidation: Tests for dual-interpreter validation (bundled + venv Python) - TestSystemCheckIntegration: Tests for system_check.py including build tools detection, platform requirements, and installation instructions - TestDependencyInstallerIntegration: Tests for dependency_installer.py including package listing and synchronization verification - TestStartupValidationFlow: E2E tests simulating the complete startup validation sequence - TestFeatureValidation: Tests verifying Insights, Roadmap, Ideation features would not produce cryptic 'code 1' errors - TestCLIValidation: Tests for CLI interfaces (help, JSON output) - TestErrorMessageQuality: Tests ensuring error messages are descriptive - TestFreshInstallSimulation: Tests simulating fresh install scenarios Tests include Python version compatibility handling for systems < 3.10. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
… Implement comprehensive environment validation system with dependency installation checks, Python environment detection, and startup validation. Add IPC handlers and frontend component to detect and handle misconfigured Python environments causing crashes in Insights, Roadmap, and Ideation features on macOS and Windows. Fixes AndyMik90#139
Resolve conflict in apps/frontend/src/preload/api/index.ts by combining: - EnvironmentAPI from AndyMik90#139 fix (process exit code 1 errors) - ClaudeCodeAPI and McpAPI from upstream 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add missing @types/semver and @radix-ui/react-popover dependencies - Add EnvironmentValidationStatus type and validation methods to ElectronAPI interface - Update StartupValidator.tsx to import type from shared types - Add validation mock implementations to browser-mock.ts - Fix system_check.py CLI to not print extra text after JSON output - Increase timeout for IPC handler tests that were timing out in parallel runs All pre-commit checks pass (TypeScript, ESLint, Python tests) All frontend tests pass (944 tests) All backend tests pass (1362 tests) Fixes AndyMik90#139 Co-Authored-By: Warp <[email protected]>
|
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR adds comprehensive environment validation capabilities spanning backend Python services (environment validation, system build tool checking, dependency installation) and a frontend startup validator UI integrated via IPC handlers. It includes extensive test coverage and updates version badges to 2.7.2-beta.10. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as StartupValidator<br/>(Renderer)
participant IPC as IPC Handler<br/>(Main)
participant PEM as PythonEnvManager<br/>(Main)
participant BV as Backend<br/>Validators<br/>(system_check.py,<br/>environment_validator.py)
participant Renderer2 as App<br/>(Renderer)
Renderer->>IPC: startValidation()
IPC->>IPC: reset validationState
IPC->>PEM: validateBuildTools()
PEM->>BV: run system_check.py
BV-->>PEM: tool status results
IPC->>Renderer: push ENV_VALIDATE_PROGRESS
IPC->>PEM: validateBothEnvironments()
PEM->>BV: run environment_validator.py<br/>(bundled & venv)
BV-->>PEM: validation results
IPC->>Renderer: push ENV_VALIDATE_PROGRESS
IPC->>IPC: compute overallSuccess,<br/>persist timestamp
IPC->>Renderer: emit ENV_VALIDATE_COMPLETE
Renderer->>Renderer2: onValidationComplete()<br/>persist state,<br/>dismiss validator
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
Summary of ChangesHello @JustYannicc, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a comprehensive solution to prevent and diagnose 'Process exited with code 1' errors, which often arise from missing Python dependencies or system build tools. It introduces a new set of backend Python scripts for environment validation and a user-friendly frontend component to guide users through resolving any detected issues. This proactive approach significantly enhances the application's robustness and user experience by providing clear, actionable feedback on environment setup. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive environment validation system to proactively address dependency and build tool issues, which is a fantastic improvement for user experience and stability. The addition of dedicated Python scripts for validation, along with a new UI component in the frontend, makes the startup process more robust and informative. The enhanced error messages that now include stderr output are also a significant step up from the generic "Process exited with code 1" errors.
However, there are a couple of critical points to address. The pnpm-lock.yaml file has been removed, which will break deterministic dependency installation with pnpm. This file should be restored or an alternative lock file provided. Additionally, the new validation logic in the main process uses execSync, which blocks the event loop and can make the application unresponsive during startup. This should be refactored to use asynchronous process execution.
Overall, this is a very valuable contribution that significantly improves the application's resilience to environment-related problems.
| except Exception: | ||
| pass |
There was a problem hiding this comment.
The broad except Exception: pass can silently swallow any error that occurs during pip list, such as a corrupted pip installation or network issues. This makes it difficult to diagnose problems with the Python environment. It would be better to at least log the exception to aid in debugging.
| except Exception: | |
| pass | |
| except Exception as e: | |
| # Consider logging the exception for better diagnostics | |
| # import logging | |
| # logging.warning(f"Failed to get packages for {python_path}: {e}") | |
| pass |
| const result = execSync( | ||
| `"${pythonPath}" "${systemCheckPath}" --json`, | ||
| { | ||
| stdio: 'pipe', | ||
| timeout: 30000, // 30 second timeout for system check | ||
| encoding: 'utf-8' | ||
| } | ||
| ); |
There was a problem hiding this comment.
The use of execSync blocks the main process's event loop. While this runs in an async IPC handler, it can still make the application unresponsive to other main-process tasks during the validation, which might take some time. It's recommended to use an asynchronous alternative like exec or spawn to keep the main process responsive.
|
|
||
| <!-- STABLE_VERSION_BADGE --> | ||
| [](https://github.com/AndyMik90/Auto-Claude/releases/tag/v2.7.1) | ||
| [](https://github.com/AndyMik90/Auto-Claude/releases/tag/v2.7.2-beta.10) |
There was a problem hiding this comment.
The "Stable" release badge incorrectly points to a beta tag (v2.7.2-beta.10) instead of the stable version it displays (2.7.1). This link should point to the v2.7.1 tag to match the badge text.
| [](https://github.com/AndyMik90/Auto-Claude/releases/tag/v2.7.2-beta.10) | |
| [](https://github.com/AndyMik90/Auto-Claude/releases/tag/v2.7.1) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/frontend/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
README.mdapps/backend/services/dependency_installer.pyapps/backend/services/environment_validator.pyapps/backend/services/system_check.pyapps/backend/spec/test_environment_validator.pyapps/backend/spec/test_system_check.pyapps/frontend/src/main/__tests__/ipc-handlers.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/main/ipc-handlers/context/utils.tsapps/frontend/src/main/ipc-handlers/environment-handlers.tsapps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.tsapps/frontend/src/main/ipc-handlers/index.tsapps/frontend/src/main/python-env-manager.tsapps/frontend/src/preload/api/index.tsapps/frontend/src/preload/api/modules/environment-api.tsapps/frontend/src/preload/api/modules/index.tsapps/frontend/src/renderer/App.tsxapps/frontend/src/renderer/components/StartupValidator.tsxapps/frontend/src/renderer/lib/browser-mock.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/shared/types/ipc.tstests/integration/__init__.pytests/integration/test_startup_validation.pytests/test_security_cache.py
🧰 Additional context used
📓 Path-based instructions (5)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/__tests__/ipc-handlers.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/preload/api/modules/environment-api.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/main/ipc-handlers/environment-handlers.tsapps/frontend/src/renderer/lib/browser-mock.tsapps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/main/ipc-handlers/index.tsapps/frontend/src/preload/api/modules/index.tsapps/frontend/src/renderer/components/StartupValidator.tsxapps/frontend/src/preload/api/index.tsapps/frontend/src/renderer/App.tsxapps/frontend/src/main/python-env-manager.tsapps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.tsapps/frontend/src/main/ipc-handlers/context/utils.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/__tests__/ipc-handlers.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/preload/api/modules/environment-api.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/main/ipc-handlers/environment-handlers.tsapps/frontend/src/renderer/lib/browser-mock.tsapps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/main/ipc-handlers/index.tsapps/frontend/src/preload/api/modules/index.tsapps/frontend/src/renderer/components/StartupValidator.tsxapps/frontend/src/preload/api/index.tsapps/frontend/src/renderer/App.tsxapps/frontend/src/main/python-env-manager.tsapps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.tsapps/frontend/src/main/ipc-handlers/context/utils.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/__tests__/ipc-handlers.test.tsapps/frontend/src/main/insights/insights-executor.tsapps/frontend/src/preload/api/modules/environment-api.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/main/ipc-handlers/environment-handlers.tsapps/frontend/src/renderer/lib/browser-mock.tsapps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/main/ipc-handlers/index.tsapps/frontend/src/preload/api/modules/index.tsapps/frontend/src/renderer/components/StartupValidator.tsxapps/frontend/src/preload/api/index.tsapps/frontend/src/renderer/App.tsxapps/frontend/src/main/python-env-manager.tsapps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.tsapps/frontend/src/main/ipc-handlers/context/utils.ts
tests/**
⚙️ CodeRabbit configuration file
tests/**: Ensure tests are comprehensive and follow pytest conventions.
Check for proper mocking and test isolation.
Files:
tests/test_security_cache.pytests/integration/__init__.pytests/integration/test_startup_validation.py
apps/backend/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
apps/backend/**/*.py: Always use the Claude Agent SDK (claude-agent-sdkpackage) for all AI interactions, never use the Anthropic API directly
Use thecreate_client()function fromapps/backend/core/client.pyto instantiate Claude SDK clients, not directClaudeSDKClientinitialization
Files:
apps/backend/spec/test_environment_validator.pyapps/backend/services/system_check.pyapps/backend/spec/test_system_check.pyapps/backend/services/environment_validator.pyapps/backend/services/dependency_installer.py
⚙️ CodeRabbit configuration file
apps/backend/**/*.py: Focus on Python best practices, type hints, and async patterns.
Check for proper error handling and security considerations.
Verify compatibility with Python 3.12+.
Files:
apps/backend/spec/test_environment_validator.pyapps/backend/services/system_check.pyapps/backend/spec/test_system_check.pyapps/backend/services/environment_validator.pyapps/backend/services/dependency_installer.py
🧠 Learnings (2)
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: When submitting PRs to the upstream AndyMik90/Auto-Claude repository, always target the `develop` branch, not `main`
Applied to files:
README.md
📚 Learning: 2025-12-30T16:38:36.276Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.276Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP for E2E testing by setting `ELECTRON_MCP_ENABLED=true` in `.env` and starting the Electron app with `npm run dev`
Applied to files:
apps/frontend/src/main/ipc-handlers/environment-handlers.ts
🧬 Code graph analysis (10)
tests/test_security_cache.py (1)
apps/backend/security/profile.py (1)
get_security_profile(42-88)
apps/frontend/src/preload/api/modules/environment-api.ts (1)
apps/frontend/src/shared/constants/ipc.ts (1)
IPC_CHANNELS(6-487)
apps/frontend/src/main/ipc-handlers/environment-handlers.ts (2)
apps/frontend/src/main/ipc-handlers/index.ts (1)
registerEnvironmentHandlers(135-135)apps/frontend/src/main/python-env-manager.ts (2)
pythonEnvManager(1194-1194)PythonEnvManager(155-1191)
apps/backend/spec/test_environment_validator.py (4)
apps/backend/services/environment_validator.py (10)
DependencyStatus(71-85)DualValidationResult(117-131)EnvironmentValidator(208-477)ValidationResult(89-113)get_bundled_python_path(139-172)get_venv_python_path(175-200)_check_python_version(292-349)_check_dependency(351-415)validate_environment(233-290)validate_dual_environment(417-477)scripts/bump-version.js (1)
status(106-106)apps/frontend/src/renderer/components/settings/utils/hookProxyFactory.ts (1)
error(18-18)tests/test_output_validator.py (1)
validator(91-93)
apps/backend/services/system_check.py (3)
apps/backend/services/dependency_installer.py (1)
main(634-732)apps/backend/services/environment_validator.py (1)
main(485-550)apps/backend/cli/main.py (1)
parse_args(48-258)
apps/frontend/src/main/ipc-handlers/index.ts (2)
apps/frontend/src/main/ipc-handlers/environment-handlers.ts (1)
registerEnvironmentHandlers(43-162)apps/frontend/src/main/python-env-manager.ts (1)
pythonEnvManager(1194-1194)
apps/frontend/src/renderer/components/StartupValidator.tsx (5)
apps/frontend/src/shared/types/ipc.ts (1)
EnvironmentValidationStatus(128-146)apps/frontend/src/preload/api/modules/environment-api.ts (1)
EnvironmentValidationStatus(8-26)apps/frontend/src/renderer/components/settings/utils/hookProxyFactory.ts (1)
error(18-18).design-system/src/lib/utils.ts (1)
cn(4-6).design-system/src/components/Button.tsx (1)
Button(10-44)
apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.ts (1)
apps/frontend/src/main/python-detector.ts (1)
parsePythonCommand(215-258)
apps/backend/services/environment_validator.py (1)
apps/backend/services/dependency_installer.py (2)
get_bundled_python_path(157-189)get_venv_python_path(192-217)
apps/backend/services/dependency_installer.py (1)
apps/backend/services/environment_validator.py (2)
get_bundled_python_path(139-172)get_venv_python_path(175-200)
🔇 Additional comments (43)
apps/frontend/src/main/ipc-handlers/context/utils.ts (1)
131-136: LGTM!Minor JSDoc formatting improvement that adds visual separation between the provider list and the
@returnstag. No functional changes.apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.test.ts (1)
70-96: Test logic and comments are clear.The second test effectively verifies that user arguments follow python base arguments in the correct order. The comments at lines 87–88 and 89 clearly articulate the verification goal, and the assertion structure directly validates the intended behavior.
apps/frontend/src/main/insights/insights-executor.ts (2)
133-133: LGTM! Excellent addition of separate stderr capture.The separate
stderrOutputaccumulator with a 5KB limit provides valuable diagnostic context for error scenarios while keeping memory usage bounded. This directly addresses issue #139 by preserving stderr details for inclusion in error messages without disrupting the existingallInsightsOutputused for rate-limit detection.Also applies to: 165-166
202-206: LGTM! Error message enhancement effectively resolves issue #139.The improved error message construction now includes stderr details when available, providing users and developers with actionable diagnostic information instead of just an exit code. The fallback to a code-only message handles cases where no stderr is captured, ensuring robust error reporting in all scenarios.
tests/test_security_cache.py (1)
100-116: Test structure is sound and follows pytest conventions.The whitespace-only changes improve readability by separating logical blocks in
test_cache_invalidation_on_file_deletion. The test correctly uses fixtures, resets state between tests withreset_profile_cache(), and properly handles filesystem timing withtime.sleep(1.0)to account for varying mtime resolution across systems.tests/integration/__init__.py (1)
1-8: Standard package initializer with clear documentation.The module-level docstring accurately describes the purpose and scope of the integration tests package. This follows Python packaging conventions.
apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts (1)
98-104: Formatting changes improve readability without altering logic.The blank line insertion (line 98) and condition reflow (lines 102–104) are whitespace-only changes that improve code clarity. The
hasIncompleteSubtaskscondition logic remains functionally identical while becoming more readable across multiple lines.apps/backend/spec/test_environment_validator.py (1)
1-35: LGTM!Comprehensive test suite with good coverage of data classes, path detection, version checking, dependency validation, and dual environment validation. The test structure is clear and well-organized.
apps/backend/services/dependency_installer.py (2)
273-282: LGTM!The
DependencyInstallerclass is well-designed with clear responsibilities, proper error handling, and good separation of concerns between installation and verification operations.
554-556:synchronizedflag does not account for version differences.The code detects version differences between interpreters (lines 548-552) and adds them to the
differenceslist, but thesynchronizedflag (lines 554-556) only checks for missing packages. This meanssynchronized=Truecan be returned even when the same package has different versions in each interpreter. The docstring definessynchronizedas "Whether both have same packages" (not versions), but this creates ambiguous semantics where the output reports "Dependencies synchronized" while simultaneously listing version mismatches. Clarify whether version differences should also setsynchronized=False, or update the docstring to explicitly exclude version mismatches from the synchronization definition.apps/backend/services/environment_validator.py (2)
208-216: LGTM!The
EnvironmentValidatorclass provides comprehensive validation of Python environments with clear error reporting, proper timeout handling, and support for both single and dual environment validation scenarios.
51-55: CORE_DEPENDENCIES correctly uses import names, not package names — this is intentional and correct.The list stores module names used in Python
importstatements (e.g.,"dotenv"), not PyPI package names (e.g.,"python-dotenv"). Since_check_dependency()validates importability via subprocess import execution (line 369), using import names is the appropriate design. This approach correctly handles dotted modules like"google.generativeai"and hyphenated packages like"graphiti-core"(which imports asgraphiti_core).apps/frontend/src/main/__tests__/ipc-handlers.test.ts (1)
229-229: LGTM!The timeout increase to 15 seconds is appropriate for IPC operations that involve file system access. This should prevent flaky test failures.
apps/frontend/src/preload/api/modules/index.ts (1)
17-17: LGTM!The new export follows the established barrel pattern for API modules.
apps/frontend/src/main/ipc-handlers/index.ts (1)
82-84: LGTM!Clean integration of environment validation handlers following the established pattern. The comment clarifies the distinction from
registerEnvHandlers(configuration) vsregisterEnvironmentHandlers(validation/health check).apps/frontend/src/renderer/App.tsx (4)
146-152: LGTM!Good use of
sessionStoragefor per-session validation state. The initialization from storage ensures the validator doesn't repeatedly show after completion within the same session.
184-200: LGTM!The effect correctly triggers the startup validator on first project load with proper guards to prevent re-triggering. All dependencies are correctly specified.
651-664: LGTM!Clear distinction between completion (persists to sessionStorage) and skip (temporary, will prompt again on next session). This is good UX design.
965-975: UseuseTranslation()hook with i18n keys for all user-facing text inStartupValidator.The component contains multiple hardcoded user-facing strings throughout (e.g., "Environment Validation", "Checking build tools...", "Missing Build Tools", "Skip for Now", "Retry Validation", etc.). Per the coding guidelines, all user-facing text must use i18n translation keys via the
useTranslation()hook with namespace prefixes (e.g., 'validator:messages.environmentValidation'). Add translation keys to the language files and refactor the component to use the hook for all displayed strings.⛔ Skipped due to learnings
Learnt from: CR Repo: AndyMik90/Auto-Claude PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-30T16:38:36.276Z Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded stringsLearnt from: CR Repo: AndyMik90/Auto-Claude PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-30T16:38:36.276Z Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)Learnt from: CR Repo: AndyMik90/Auto-Claude PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-30T16:38:36.276Z Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React componentsLearnt from: CR Repo: AndyMik90/Auto-Claude PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-30T16:38:36.276Z Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported languageapps/frontend/src/preload/api/index.ts (1)
13-13: LGTM!The EnvironmentAPI integration follows the established pattern used by other API modules (GitHubAPI, DebugAPI, etc.). The import, interface extension, factory spread, and exports are all consistent with existing code.
Also applies to: 29-29, 47-47, 67-67, 85-85
apps/frontend/src/shared/constants/ipc.ts (1)
156-162: LGTM!The new IPC channel constants follow the established naming conventions and are logically grouped with other environment-related channels. The lifecycle coverage (status, start, progress, complete, error) is complete and well-documented.
apps/frontend/src/renderer/lib/browser-mock.ts (1)
233-258: LGTM!The environment validation mock implementation follows the established pattern for other mocked operations. The return values correctly represent a "validation complete" state, and the event listeners appropriately return no-op cleanup functions consistent with other mock listeners in this file.
apps/frontend/src/preload/api/modules/environment-api.ts (1)
45-62: LGTM!The
createEnvironmentAPIfactory correctly wires up IPC channels for operations and event listeners. The pattern matches other API modules in the codebase.apps/frontend/src/shared/types/ipc.ts (2)
127-146: LGTM!The
EnvironmentValidationStatusinterface is well-structured with clear nullable result types for both build tools and environment validation stages. This serves as the canonical type definition that should be imported by other modules.
770-778: LGTM!The ElectronAPI extensions for environment validation follow the established patterns for both operations (returning
Promise<IPCResult<T>>) and event listeners (returning cleanup functions).apps/backend/spec/test_system_check.py (1)
1-713: Good test coverage overall.The test suite comprehensively covers:
- Dataclass default values and populated states
- Platform-specific requirements and instructions
- Tool detection with version extraction
- Error handling (timeouts, missing tools, non-zero exits)
- Edge cases (stderr output, multiline paths)
The mocking strategy with
subprocess.runis appropriate for testing system tool detection without requiring actual tool installations.apps/frontend/src/main/ipc-handlers/environment-handlers.ts (2)
43-161: Good overall structure for the IPC handler registration.The handler properly:
- Guards against concurrent validation
- Resets state before starting
- Sends progress updates to the renderer
- Handles errors gracefully with state updates and error events
- Returns consistent
IPCResultresponses
100-101: Timeouts are already implemented in the underlyingPythonEnvManagermethods.validateBuildTools()uses a 30-second timeout for the system check process, andvalidateBothEnvironments()uses a 60-second timeout for environment validation, both viaexecSyncoptions. No action needed.tests/integration/test_startup_validation.py (2)
526-560: Good test coverage for the full validation flow.The
TestStartupValidationFlowclass properly tests the E2E validation sequence, verifies structured results, and ensures detailed error messages replace cryptic "code 1" errors. This aligns well with the PR's objective.
614-684: Effective regression tests for the core issue.The
TestFeatureValidationclass directly tests that subprocess errors are properly captured with meaningful messages instead of just exit codes. This provides good regression coverage for issue #139.apps/frontend/src/renderer/components/StartupValidator.tsx (3)
350-355: Using array index as React key is acceptable here.Since the error arrays are static after validation completes and items don't have stable IDs, using index as key is acceptable for this use case.
Also applies to: 362-366
27-47: Well-implemented reduced motion hook.The
useReducedMotionhook properly handles SSR safety, media query changes, and cleanup. Good accessibility practice.
84-107: Good error handling in startValidation.The async validation properly handles both structured errors from IPC and unexpected exceptions, updating UI state appropriately in each case.
apps/backend/services/system_check.py (5)
84-102: Well-designed dataclasses with sensible defaults.The
ToolStatusandSystemCheckResultdataclasses use proper type hints and field defaults. Good use offield(default_factory=list)for mutable defaults.
205-238: Good subprocess error handling in_check_tool.The method properly handles
FileNotFoundError,TimeoutExpired, and generic exceptions, providing meaningful error messages in each case. The 10-second timeout is reasonable.
273-291: Cross-platform path resolution is correct.The
_get_tool_pathmethod correctly useswhereon Windows andwhichon Unix systems. Good handling of the subprocess call with appropriate timeout.
346-352: CLI platform choices don't match internal constants.The CLI accepts lowercase platform names (
macos,windows,linux) but the internalPLATFORM_REQUIREMENTSusesDarwin,Windows,Linux. The mapping (lines 374-378) handles this correctly, but the help text could be clearer about accepted values matching the actual OS identifiers.
166-191: Clean implementation ofvalidate_build_tools.The method follows a clear pattern: collect tool statuses, track missing tools, generate instructions, and determine overall success. Good separation of concerns.
apps/frontend/src/main/python-env-manager.ts (5)
15-125: Well-structured validation logger with cross-platform support.The
EnvValidationLoggerclass properly handles different platforms for log directories, lazy initialization, and provides useful logging methods for validation sessions. Good error handling that doesn't throw on failures.
853-868: Improved error reporting for pip failures.The enhanced error handling now captures both stdout and stderr, truncates appropriately, and provides detailed error messages including exit code. This directly addresses the "Process exited with code 1" issue from PR #139.
486-514: Good fallback handling for non-zero exit codes.The code properly handles the case where the system check script exits with code 1 but still returns valid JSON output. This allows the frontend to display meaningful error information.
391-439: Solid implementation ofvalidateBuildTools.The method has good fallback logic for finding the script in both development and packaged contexts, and returns a well-structured result even when the validator isn't found.
313-380: Comprehensive logging throughoutvalidateBothEnvironments.The method logs at each decision point, making debugging easier. Good use of the validation logger to track session state.
| VERIFICATION_PACKAGES = [ | ||
| "claude_agent_sdk", | ||
| "dotenv", | ||
| "pydantic", | ||
| ] |
There was a problem hiding this comment.
Package name mismatch: dotenv vs python-dotenv.
The VERIFICATION_PACKAGES list includes "dotenv", but the actual PyPI package is python-dotenv. When checking via pip list, the package name will appear as python-dotenv, not dotenv. This could cause false negatives in synchronization checks.
🔎 Proposed fix
VERIFICATION_PACKAGES = [
"claude_agent_sdk",
- "dotenv",
+ "python-dotenv",
"pydantic",
]🤖 Prompt for AI Agents
In apps/backend/services/dependency_installer.py around lines 52 to 56, the
VERIFICATION_PACKAGES list wrongly uses "dotenv" instead of the PyPI package
name "python-dotenv", causing pip-based checks to miss the installed package;
update the list entry from "dotenv" to "python-dotenv" (or include both names if
code elsewhere expects the short name) so the package name used for verification
matches the actual pip package name returned by pip list.
| def get_bundled_python_path() -> Path | None: | ||
| """ | ||
| Get the path to the bundled Python interpreter. | ||
| Returns: | ||
| Path to bundled Python, or None if not found. | ||
| """ | ||
| system = platform.system() | ||
|
|
||
| if system == "Darwin": # macOS | ||
| candidates = [ | ||
| Path("/Applications/Auto-Claude.app/Contents/Resources/python/bin/python3"), | ||
| Path.home() | ||
| / "Applications/Auto-Claude.app/Contents/Resources/python/bin/python3", | ||
| ] | ||
| elif system == "Windows": | ||
| appdata_local = Path.home() / "AppData/Local" | ||
| candidates = [ | ||
| appdata_local / "Programs/auto-claude-ui/resources/python/python.exe", | ||
| Path.home() | ||
| / "AppData/Roaming/../Local/Programs/auto-claude-ui/resources/python/python.exe", | ||
| ] | ||
| else: # Linux | ||
| candidates = [ | ||
| Path("/opt/auto-claude/python/bin/python3"), | ||
| Path.home() / ".local/share/auto-claude/python/bin/python3", | ||
| ] | ||
|
|
||
| for candidate in candidates: | ||
| if candidate.exists(): | ||
| return candidate | ||
|
|
||
| return None | ||
|
|
||
|
|
||
| def get_venv_python_path() -> Path | None: | ||
| """ | ||
| Get the path to the venv Python interpreter. | ||
| Returns: | ||
| Path to venv Python, or None if not found. | ||
| """ | ||
| system = platform.system() | ||
|
|
||
| if system == "Darwin": # macOS | ||
| venv_path = ( | ||
| Path.home() | ||
| / "Library/Application Support/auto-claude-ui/python-venv/bin/python" | ||
| ) | ||
| elif system == "Windows": | ||
| venv_path = ( | ||
| Path.home() | ||
| / "AppData/Roaming/auto-claude-ui/python-venv/Scripts/python.exe" | ||
| ) | ||
| else: # Linux | ||
| venv_path = Path.home() / ".config/auto-claude-ui/python-venv/bin/python" | ||
|
|
||
| if venv_path.exists(): | ||
| return venv_path | ||
|
|
||
| return None |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Code duplication: get_bundled_python_path and get_venv_python_path are duplicated.
These functions are identical to those in environment_validator.py. Consider extracting them to a shared utility module to avoid maintenance burden and potential drift.
🔎 Suggested refactor
Create a shared module apps/backend/services/python_paths.py:
# python_paths.py
from pathlib import Path
import platform
def get_bundled_python_path() -> Path | None:
# ... existing implementation
def get_venv_python_path() -> Path | None:
# ... existing implementationThen import in both files:
# In dependency_installer.py and environment_validator.py
+from python_paths import get_bundled_python_path, get_venv_python_path
-
-def get_bundled_python_path() -> Path | None:
- ...Committable suggestion skipped: line range outside the PR's diff.
| try: | ||
| package_list = json.loads(result.stdout) | ||
| for pkg in package_list: | ||
| packages[pkg.get("name", "")] = pkg.get("version", "") | ||
| except json.JSONDecodeError: | ||
| pass | ||
|
|
||
| except Exception: | ||
| pass | ||
|
|
||
| return packages |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent exception handling hides errors.
The _get_packages_dict method silently swallows JSONDecodeError and generic Exception. This makes debugging difficult when package listing fails. Consider logging the error or at least capturing it for diagnostics.
🔎 Proposed fix
if result.returncode == 0:
try:
package_list = json.loads(result.stdout)
for pkg in package_list:
packages[pkg.get("name", "")] = pkg.get("version", "")
except json.JSONDecodeError:
- pass
+ # pip output was not valid JSON - likely a warning message mixed in
+ pass
- except Exception:
- pass
+ except subprocess.TimeoutExpired:
+ # pip list timed out - return empty dict
+ pass
+ except FileNotFoundError:
+ # Python interpreter not found - return empty dict
+ pass
+ except Exception as e:
+ # Log unexpected errors for debugging but don't fail
+ import logging
+ logging.debug(f"Failed to get packages from {python_path}: {e}")
return packagesCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/backend/services/dependency_installer.py around lines 616 to 626, the
code silently swallows JSONDecodeError and a generic Exception when parsing
package_list; change it to catch JSONDecodeError and the broad Exception but log
the exception details (including the exception message and relevant context like
the command output or package name) using the module/process logger before
returning, and avoid completely swallowing unexpected exceptions — either
re-raise after logging or return an explicit empty dict while logging the
failure so diagnostics are available.
| try: | ||
| # Build import check script | ||
| # Handle dotted module names (e.g., "google.generativeai") | ||
| import_name = module_name.split(".")[0] | ||
| # Use string concatenation to avoid f-string escaping issues | ||
| version_code = ( | ||
| """ | ||
| import sys | ||
| import json | ||
| try: | ||
| import """ | ||
| + import_name | ||
| + """ | ||
| version = getattr(""" | ||
| + import_name | ||
| + """, '__version__', 'unknown') | ||
| print(json.dumps({"installed": True, "version": version})) | ||
| except ImportError as e: | ||
| print(json.dumps({"installed": False, "error": str(e)})) | ||
| """ | ||
| ) |
There was a problem hiding this comment.
Code injection risk with custom dependency names.
The _check_dependency method constructs Python code using string concatenation with import_name derived from user-provided module_name. While the subprocess isolation and .split(".")[0] provide some protection, a malicious dependency name like "__import__('os').system('cmd')" could potentially be exploited if custom dependencies are provided.
Consider sanitizing import_name to ensure it only contains valid Python identifier characters.
🔎 Proposed fix
# Build import check script
# Handle dotted module names (e.g., "google.generativeai")
import_name = module_name.split(".")[0]
+
+ # Sanitize import_name to prevent code injection
+ if not import_name.isidentifier():
+ return DependencyStatus(
+ name=module_name,
+ installed=False,
+ error=f"Invalid module name: {module_name}"
+ )
+
# Use string concatenation to avoid f-string escaping issues| # Common patterns for version extraction | ||
| import re | ||
|
|
||
| # Try to find version pattern like "X.Y.Z" or "X.Y" | ||
| version_pattern = re.search(r"(\d+\.\d+(?:\.\d+)?)", output) | ||
| if version_pattern: | ||
| return version_pattern.group(1) | ||
|
|
||
| # If no version found, return first 50 chars of output | ||
| return output[:50] if len(output) > 50 else output |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Import inside function body.
The re module is imported inside _extract_version. While this works, it's unconventional and adds slight overhead on each call. Consider moving to module level.
🔎 Suggested fix
import json
import platform
+import re
import subprocess
import sys
from dataclasses import dataclass, field
from typing import Any
# ... later in the file ...
def _extract_version(self, output: str, tool_name: str) -> str:
"""..."""
# Common patterns for version extraction
- import re
-
# Try to find version pattern like "X.Y.Z" or "X.Y"
version_pattern = re.search(r"(\d+\.\d+(?:\.\d+)?)", output)🤖 Prompt for AI Agents
In apps/backend/services/system_check.py around lines 252 to 261, the re module
is imported inside the _extract_version function which is unconventional and
causes unnecessary overhead on each call; move the import to the module level
(add "import re" to the top of the file alongside other imports) and remove the
local "import re" line from inside the function so the function uses the
top-level import.
| # Check Python version - codebase requires 3.10+ for type syntax | ||
| PYTHON_VERSION_OK = sys.version_info >= (3, 10) | ||
| pytestmark = pytest.mark.skipif( | ||
| not PYTHON_VERSION_OK, | ||
| reason="Tests require Python 3.10+ due to modern type annotations in codebase" | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Python version check inconsistency with requirements.
The test file checks for Python 3.10+ (line 37), but the codebase requires Python 3.12+ as indicated by MIN_PYTHON_VERSION = (3, 12) tested in line 241. Consider aligning the skip condition with the actual minimum requirement to avoid running tests on unsupported Python versions.
🔎 Suggested fix
# Check Python version - codebase requires 3.10+ for type syntax
-PYTHON_VERSION_OK = sys.version_info >= (3, 10)
+PYTHON_VERSION_OK = sys.version_info >= (3, 12)
pytestmark = pytest.mark.skipif(
not PYTHON_VERSION_OK,
- reason="Tests require Python 3.10+ due to modern type annotations in codebase"
+ reason="Tests require Python 3.12+ (minimum supported version for Auto-Claude)"
)🤖 Prompt for AI Agents
In tests/integration/test_startup_validation.py around lines 36 to 41, the skip
condition checks for Python >= 3.10 but the project enforces MIN_PYTHON_VERSION
= (3, 12); update the check to match the actual minimum (e.g., change the tuple
to (3, 12)) or, better, import and use the canonical MIN_PYTHON_VERSION constant
from the codebase so the pytestmark skip condition automatically stays in sync
with the real requirement.
| @pytest.fixture | ||
| def temp_python_script() -> Generator[Path, None, None]: | ||
| """Create a temporary Python script for validation testing.""" | ||
| with tempfile.NamedTemporaryFile( | ||
| mode="w", suffix=".py", delete=False | ||
| ) as f: | ||
| f.write( | ||
| """#!/usr/bin/env python3 | ||
| import sys | ||
| print(f"{sys.version_info.major}.{sys.version_info.minor}.{sys.version_info.micro}") | ||
| sys.exit(0) | ||
| """ | ||
| ) | ||
| script_path = Path(f.name) | ||
|
|
||
| yield script_path | ||
|
|
||
| # Cleanup | ||
| if script_path.exists(): | ||
| script_path.unlink() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fixture lacks cleanup on test failure.
The temp_python_script fixture creates a file but cleanup only runs after yield. If an exception occurs before yield, the file might not be cleaned up. Consider using try/finally for more robust cleanup.
🔎 Suggested fix
@pytest.fixture
def temp_python_script() -> Generator[Path, None, None]:
"""Create a temporary Python script for validation testing."""
+ script_path = None
+ try:
with tempfile.NamedTemporaryFile(
mode="w", suffix=".py", delete=False
) as f:
f.write(
"""#!/usr/bin/env python3
import sys
print(f"{sys.version_info.major}.{sys.version_info.minor}.{sys.version_info.micro}")
sys.exit(0)
"""
)
script_path = Path(f.name)
- yield script_path
+ yield script_path
+ finally:
+ # Cleanup
+ if script_path and script_path.exists():
+ script_path.unlink()
-
- # Cleanup
- if script_path.exists():
- script_path.unlink()Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tests/integration/test_startup_validation.py around lines 109 to 128, the
temp_python_script fixture can leak the temporary file if an exception occurs
before the yield; wrap the file creation and yield in a try/finally so that the
finally block always removes the file if it exists (or alternatively use
pytest's tmp_path fixture), i.e. create the temp file, set script_path, then
try: yield script_path finally: if script_path and script_path.exists():
script_path.unlink().
| @pytest.fixture | ||
| def mock_python_paths(temp_dir: Path) -> dict[str, Path]: | ||
| """Create mock Python interpreter paths for testing.""" | ||
| # Create mock bundled Python | ||
| bundled_dir = temp_dir / "bundled" / "bin" | ||
| bundled_dir.mkdir(parents=True) | ||
| bundled_python = bundled_dir / "python3" | ||
|
|
||
| # Create mock venv Python | ||
| venv_dir = temp_dir / "venv" / "bin" | ||
| venv_dir.mkdir(parents=True) | ||
| venv_python = venv_dir / "python" | ||
|
|
||
| # Create executable scripts that mimic Python | ||
| python_script = f"""#!/bin/bash | ||
| echo "3.12.0" | ||
| exit 0 | ||
| """ | ||
| bundled_python.write_text(python_script) | ||
| bundled_python.chmod(0o755) | ||
| venv_python.write_text(python_script) | ||
| venv_python.chmod(0o755) | ||
|
|
||
| return { | ||
| "bundled": bundled_python, | ||
| "venv": venv_python, | ||
| } |
There was a problem hiding this comment.
Platform-specific fixture only works on Unix.
The mock_python_paths fixture creates bash scripts (lines 145-148) that won't work on Windows. This limits test portability.
🔎 Suggested cross-platform fix
# Create executable scripts that mimic Python
- python_script = f"""#!/bin/bash
-echo "3.12.0"
-exit 0
-"""
+ if sys.platform == 'win32':
+ python_script = '@echo off\necho 3.12.0'
+ bundled_python = bundled_dir.parent / "Scripts" / "python.exe"
+ venv_python = venv_dir.parent / "Scripts" / "python.exe"
+ bundled_python.parent.mkdir(parents=True, exist_ok=True)
+ venv_python.parent.mkdir(parents=True, exist_ok=True)
+ bundled_python.with_suffix('.bat').write_text(python_script)
+ venv_python.with_suffix('.bat').write_text(python_script)
+ else:
+ python_script = """#!/bin/bash
+echo "3.12.0"
+exit 0
+"""
bundled_python.write_text(python_script)
bundled_python.chmod(0o755)
venv_python.write_text(python_script)
venv_python.chmod(0o755)Committable suggestion skipped: line range outside the PR's diff.
| def test_environment_validator_cli_help(self): | ||
| """Environment validator CLI shows help.""" | ||
| mod = _get_environment_validator_module() | ||
| main = mod.main | ||
| import io | ||
| from contextlib import redirect_stdout | ||
|
|
||
| with patch("sys.argv", ["environment_validator.py", "--help"]): | ||
| with pytest.raises(SystemExit) as exc_info: | ||
| main() | ||
| # --help exits with 0 | ||
| assert exc_info.value.code == 0 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused import in test method.
Lines 699-700 import io and redirect_stdout but they're not used in test_environment_validator_cli_help. This creates dead code.
🔎 Suggested fix - remove unused imports
def test_environment_validator_cli_help(self):
"""Environment validator CLI shows help."""
mod = _get_environment_validator_module()
main = mod.main
- import io
- from contextlib import redirect_stdout
with patch("sys.argv", ["environment_validator.py", "--help"]):
with pytest.raises(SystemExit) as exc_info:
main()
# --help exits with 0
assert exc_info.value.code == 0🤖 Prompt for AI Agents
In tests/integration/test_startup_validation.py around lines 695 to 706, the
test_environment_validator_cli_help function imports io and redirect_stdout on
lines 699-700 but never uses them; remove these two unused imports to eliminate
dead code and keep the test clean. Ensure no other references to io or
redirect_stdout remain in that test before committing the change.
| def test_environment_validator_cli_json_output(self): | ||
| """Environment validator CLI supports JSON output.""" | ||
| mod = _get_environment_validator_module() | ||
| main = mod.main | ||
| import io | ||
| from contextlib import redirect_stdout | ||
|
|
||
| with patch( | ||
| "sys.argv", | ||
| ["environment_validator.py", "--python", sys.executable, "--json"], | ||
| ): | ||
| output = io.StringIO() | ||
| with redirect_stdout(output): | ||
| exit_code = main() | ||
|
|
||
| output_str = output.getvalue() | ||
| # Should be valid JSON | ||
| try: | ||
| data = json.loads(output_str) | ||
| assert "success" in data | ||
| assert "python_path" in data | ||
| except json.JSONDecodeError: | ||
| pytest.fail(f"Invalid JSON output: {output_str}") | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate imports inside test methods.
Lines 732-733 repeat imports that were already done in test_environment_validator_cli_help. Consider moving shared imports to module level or the test class.
🤖 Prompt for AI Agents
In tests/integration/test_startup_validation.py around lines 728 to 751, the
test method test_environment_validator_cli_json_output re-imports modules
(specifically "import io" and "from contextlib import redirect_stdout") that are
already imported in test_environment_validator_cli_help; remove these duplicate
imports from the method and instead place the shared imports at the module top
or in the test class scope so both tests reuse the same imports, updating any
references if needed.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "claude_agent_sdk", | ||
| "dotenv", | ||
| "pydantic", | ||
| ] |
There was a problem hiding this comment.
Package name mismatch causes false missing dependency reports
The VERIFICATION_PACKAGES list includes "dotenv" but the actual PyPI package name is python-dotenv (as shown in requirements.txt). The verify_synchronization method compares against package names from pip list --format=json, which returns python-dotenv. Since "python-dotenv" == "dotenv" evaluates to false, the verification will incorrectly report dotenv as missing in the differences list even when the package is properly installed. The entry should be "python-dotenv" to match the pip package name.
MikeeBuilds
added
bug
4 participants
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
It fixes #139 errors that were due to missing dependencies.
Related Issue
#139
Type of Change
Area
Commit Message Format
Follow conventional commits: `fix: Fix#139 "Process exited with code 1". It also adds env checks so it cannot happen again.
Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemChecklist
developbranchCI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: No
Details:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.